Allow caller to retrieve the URL used for the request#56
Open
lfoppiano wants to merge 5 commits into
Open
Conversation
09536d8 to
d6b90e3
Compare
Author
|
Ok. If I have done it correctly, the Below the different records before and after this change. One of the hops is skip, so this might be something to fix. Before this fix: After this fix: |
sebastian-nagel
requested changes
May 22, 2026
sebastian-nagel
left a comment
There was a problem hiding this comment.
Thanks, @lfoppiano! Looks generally good.
Two major points to consider
- eventually, drop the raw URL, which would reduce the number of modified classes significantly
- might move some methods to the interface
See inline comments for details.
70872b9 to
ff1fd25
Compare
Author
|
Thanks! I wasn't expecting this PR to be so close to the completion. I've made all modifications. I still haven't figure out why the robots.txt is not visited in comment: #56 (comment), though |
Author
|
Following up my previous comment, after setting the log level to DEBUG I've got the following. main/master this branch: I cannot figure out what is going on and why the robots.txt is not visited in this branch. |
I cannot see any differences, the robots.txt is visited in both variants:
|
Author
|
Sorry, the difference is in the resulting cdx / warc file, where the record is not present in our branch. master/main/cc: this branch: I probably need to debug it 😅 |
Author
|
I think I found why in this branch we don't have There is a hit on a cache to fetch the robotsRules at FetcherThread:436: fit = {FetchItem@6480} this reach HttpRobotRulesParser.getRobotRuleSet(...): In master, the cache is not hit, because the resolved url is I still don't know what makes this latest visiting to |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR (is one part of #54 covers NUTCH-3173 for okhttp-protocol and attempt to solve the problem in a generic way.
We add a new method in the Response.java interface contract
getRawUrl()which returns the URL that was initially provided by the caller.getUrl()would return the actual URL used for the request.